Skip to content
This repository was archived by the owner on Dec 26, 2019. It is now read-only.

Allow costum value parsing #61

Merged
merged 2 commits into from
Mar 11, 2016
Merged

Allow costum value parsing #61

merged 2 commits into from
Mar 11, 2016

Conversation

@lazybensch
Copy link
Contributor

This PR is related to #58. we too need the input value for our datepicker to sometimes be something else then a date. But I understand that using the Date constructor internally is unreliable. With this PR I simply introduce an API for the user to plug in a custom parser for the date string. This way anyone can decide by him/herself how to process the dates for a particular picker.

to solve @maladon's issue, he now could do:

{{bootstrap-datepicker value=expiresAt customParser=(action myParser)}}

and inside his code:

actions: {
  myParser(value) {
    return new Date(value);
  }
}

The responsibility for how to parse is now in user land. If you want to, you could do fancy stuff with moment.js or other libraries not available from inside bootstrap-datepicker

@et
Copy link

et commented Oct 2, 2015

This would we be sweet since I've been doing this with an observer

onDatePickerValueChanged: Ember.observer('datePickerValue', function() {
})

which isn't the cleanest.

@lazybensch
Copy link
Contributor Author

@soulim any thoughts on this? should i rebase or you wanna discuss/close this PR?

@soulim
Copy link
Owner

soulim commented Mar 11, 2016

I'm sorry for delay @lazybensch. Your idea and PR looks great! 👍 Could you please rebase the branch? I'll be happy to merge it as soon as you finish with rebase.

Thank you! ❤️

@lazybensch lazybensch force-pushed the allow-costum-value-parsing branch from d18f02f to 67b7738 Compare March 11, 2016 09:54
@lazybensch
Copy link
Contributor Author

no worries @soulim. rebased the branch. CI is broken (on master though).
looks mergable to me.

@soulim
Copy link
Owner

soulim commented Mar 11, 2016

Thank a lot! Don't worry about CI. It has nothing to your PR.

soulim added a commit that referenced this pull request Mar 11, 2016
@soulim
Merge pull request #61 from lazybensch/allow-costum-value-parsing
1e5df18 
Allow costum value parsing
@soulim soulim merged commit 1e5df18 into soulim:master Mar 11, 2016
@soulim
Copy link
Owner

soulim commented Mar 11, 2016

Wish you a wonderful day! 😄

@jelhan
Copy link
Contributor

jelhan commented Aug 15, 2016

@lazybensch @soulim: The test ''resets date when input is cleared'' introduced by this PR hasn't ever passed. At least if I git checkout 1e5df180bb1323bd7bf73c7f440760921b3fe708, update ember to 1.13.13 and run tests against it fails.

jelhan
jelhan reviewed Aug 15, 2016
View reviewed changes
tests/unit/components/bootstrap-datepicker-test.js
this.$().val('');
this.$().trigger('input');

assert.equal(this.$().datepicker('getDate'), null, 'value is reset when input is cleared');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this test for? It looks like testing bootstrap-datepicker behavior but not this addon. Shouldn't the expected value be this.get('value')?

jelhan added a commit to jelhan/ember-cli-bootstrap-datepicker that referenced this pull request Aug 15, 2016
@jelhan
Fixes broken test introduced by soulim#61
32e30e7
@jelhan jelhan mentioned this pull request Aug 15, 2016
Merged
soulim added a commit that referenced this pull request Aug 15, 2016
@soulim
Merge pull request #84 from jelhan/fix-reset-test
43683d6 
Fixes broken test introduced by #61
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lazybensch @et @soulim @jelhan